Skip to content

Fix guides/security-analyzer: retention silently never runs, and the analyzer is never started - #599

Merged
lukekim merged 1 commit into
spiceai:trunkfrom
claudespice:fix/security-analyzer-retention
Aug 21, 2026
Merged

Fix guides/security-analyzer: retention silently never runs, and the analyzer is never started#599
lukekim merged 1 commit into
spiceai:trunkfrom
claudespice:fix/security-analyzer-retention

Conversation

@claudespice

Copy link
Copy Markdown
Contributor

Summary

Two problems, both of which leave a reader following the guide with something that does not work:

1. The retention config is a silent no-op. The guide's spicepod sets

retention_check_enabled: true
retention_period: 30d

with no retention_check_interval. RetentionBuilder::build() bails out with let check_interval = self.check_interval?; — so with the interval unset it returns None and the retention task is never started. No error, no warning. Because the guide also uses refresh_mode: append, nothing else evicts old rows, so the stated behavior ("keeps a month of history") never happens and the accelerator grows without bound. Every other recipe in the cookbook that configures retention (retention/, distributed/, http/, json_strings/, architectures/microservice, architectures/sidecar) already sets retention_check_interval; this guide is the only one that does not.

2. The analyzer is never started. The guide builds analyzer.py (with a uv script header) and then jumps straight to "Testing the System" — no step ever runs the runtime or the script. Added a short "Running the Analyzer" step using the same uv run convention as the other Python recipes.

Verified against

spiceai/spiceai at trunk:

  • crates/runtime-table/src/accelerated/mod.rs:2306-2311build() returns None when check_interval is None, before any filter is assembled
  • crates/runtime-component/src/dataset/mod.rs:591-606retention_check_interval() is None unless configured; there is no default
  • crates/spicepod/src/acceleration/mod.rs:535,670 — the field defaults to None
  • crates/runtime/src/datafusion/mod.rs:2945-2952 — the only construction site for dataset retention

The other model/dataset settings in the guide check out: tools and system_prompt are valid unprefixed model params (crates/runtime/src/model/params/common.rs:38-40), and time_format: timestamptz is valid (crates/runtime-component/src/dataset/mod.rs:188).

Evidence

Reproduced both states on runtime v2.1.5 with a minimal append dataset (time_column: ts, retention_period: 1m, retention_check_enabled: true).

Without retention_check_interval — rows three minutes past a one-minute retention period are still there, and the runtime logged no retention activity at all:

now=1787142520
+-------+------------+------------+
|   n   |   oldest   |   newest   |
| int64 |    int64   |    int64   |
+-------+------------+------------+
| 4     | 1787142338 | 1787142341 |
+-------+------------+------------+

$ grep '\[retention\]' run.log
(no [retention] lines at all)

With retention_check_interval: 15s added — the retention loop starts, and the registration line even advertises it:

INFO runtime::init::dataset: Dataset events registered (file:data.csv), acceleration (arrow, append, 30s refresh, 15s retention), results cache enabled. duration_ms=0
INFO runtime::accelerated_table::retention: [retention] Evicting data for events where ts < 2026-08-19T12:27:50+00:00
INFO runtime::accelerated_table::retention: [retention] Evicted 0 records for events
INFO runtime::accelerated_table::retention: [retention] Evicting data for events where ts < 2026-08-19T12:28:05+00:00

The guide itself needs Postgres and an OpenAI key, so it was not run end to end; the retention behavior was proven with the isolated repro above.

…check_interval

The guide sets retention_check_enabled and retention_period but no
retention_check_interval. RetentionBuilder::build() returns None when
check_interval is unset, so the retention task never starts — silently, with no
warning. With refresh_mode: append nothing else evicts old rows, so the
guide's claim that retention_period keeps a month of history does not hold.

Add retention_check_interval: 1h, say why both settings are needed, and add
the missing step that starts the runtime and runs analyzer.py — the guide
defines the script but never tells the reader to run it.
@claudespice claudespice added bug Something isn't working documentation Improvements or additions to documentation labels Aug 19, 2026
@claudespice claudespice self-assigned this Aug 19, 2026
@lukekim
lukekim merged commit 6826a71 into spiceai:trunk Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants